Feat: saving last block query for next fetching#716
Feat: saving last block query for next fetching#716huvny-de wants to merge 6 commits intonamada-net:mainfrom
Conversation
emccorson
added
the
needs discussion in issue
mateuszjasiuk
left a comment
mateuszjasiuk
left a comment
There was a problem hiding this comment.
Thanks! I think this is a good start for shielded sync improvements. Left few comments :)
| ); | ||
| })(); | ||
|
|
||
| const LATEST_SYNCED_HEIGHT_STORAGE = "latest-synced-height" |
There was a problem hiding this comment.
I think last block info should be stored in extension local storage.
https://github.com/anoma/namada-interface/blob/ec3dec8070219f29ccf95e8a50c880da3f032566/apps/extension/src/storage/LocalStorage.ts#L86
packages/integrations/src/Namada.ts
Outdated
| await this._namada?.shieldedSync({ startHeight, lastHeight }); | ||
| } | ||
|
|
||
| public async queryLastBlock(): Promise<number | undefined> { |
There was a problem hiding this comment.
Once you use extension local storage this function is not needed anymore :)
packages/shared/lib/src/query.rs
Outdated
| None, | ||
| None, | ||
| start_query_height, | ||
| last_query_height, |
There was a problem hiding this comment.
I'm pretty sure Namada will fetch up to last block if you pass None.
| const startHeightStorage = Number(localStorage.getItem(LATEST_SYNCED_HEIGHT_STORAGE)); | ||
| const startHeight = startHeightStorage ? startHeightStorage : undefined; | ||
| await namada.sync(startHeight,lastHeight); | ||
| if(lastHeight){ |
There was a problem hiding this comment.
Looks like formatting is a bit off. Please run yarn prepare in the root of monorepo to setup husky.
packages/shared/lib/src/query.rs
Outdated
|
|
||
| let _ = shielded.save().await?; | ||
|
|
||
| let start_query_height = match start_height { |
There was a problem hiding this comment.
You can also just start_height.map(BlockHeight) - should work :)
There was a problem hiding this comment.
Haha. Thanks for your suggestion. I'm a newbie on Rust
mateuszjasiuk
added
improvement
App: Extension
Lib: SDK
and removed
needs discussion in issue
|
Hi @mateuszjasiuk , Thanks for your comment. I fixed all of it. Please take a look on that. |
mateuszjasiuk
left a comment
mateuszjasiuk
left a comment
There was a problem hiding this comment.
In general it looks good. I've tested it locally and it seems to be working nicely :)
There are few more things to change but they should be easy.
First, linting errors (you can run yarn lint in the root of the monorepo to see them):
@namada/extension
|
| /home/mj/Projects/heliax/namada-interface/apps/extension/src/background/keyring/keyring.ts
| 819:3 error Missing return type on function @typescript-eslint/explicit-function-return-type
|
| ✖ 1 problem (1 error, 0 warnings)
|
| `yarn lint:ci` failed with exit code 1
@namada/sdk
|
| /home/mj/Projects/heliax/namada-interface/packages/sdk/src/rpc/rpc.ts
| 215:3 warning Missing JSDoc @param "start_height" declaration jsdoc/require-param
|
| ✖ 1 problem (0 errors, 1 warning)
| 0 errors and 1 warning potentially fixable with the `--fix` option.Also, we need to reset lastestSyncBlock every time we import new account. The reason is we might import an account that was used for shielding in block 10, but latestSyncBlock is 20 or whatever. So to get accurate context we need to sync from the beginning unfortunately :(
| ?.lastestSyncBlock; | ||
| const lastHeight = await this.queryLastBlock(); | ||
|
|
||
| if (startHeight !== undefined && lastHeight) { |
There was a problem hiding this comment.
I would move this "if" after the shieldedSync call. This way you can be sure that we completed the sync before saving lastBlock info
3 participants
Issue
#662
Solution
Depend on new version of SDK is Namada 0.32.0, to minimize block fetching during shielded syncing, the approach involves passing both the start_query_height and last_query_height parameters fetch function, thus restricting fetching to a specified range. To ensure the correct values for these parameters, the following solution can be implemented: